Skip to content

Fix: Add API for Orion in Scorpio and update Orion to fit the changes, allowing it to build successfully.#1301

Merged
Ivanbeethoven merged 1 commit into
gitmono-dev:mainfrom
yyjeqhc:fix_orion_build
Aug 3, 2025
Merged

Fix: Add API for Orion in Scorpio and update Orion to fit the changes, allowing it to build successfully.#1301
Ivanbeethoven merged 1 commit into
gitmono-dev:mainfrom
yyjeqhc:fix_orion_build

Conversation

@yyjeqhc

@yyjeqhc yyjeqhc commented Aug 1, 2025

Copy link
Copy Markdown
Contributor

Part of #1274
scorpio里面添加api用于orion来创建拉取任务和获取任务状态,并修改orion进行适配

@vercel

vercel Bot commented Aug 1, 2025

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
mega ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 3, 2025 8:17am

@yyjeqhc yyjeqhc changed the title fix: orion now can build ok Fix: Add API for Orion in Scaorio and update Orion to fit the changes, allowing it to build successfully. Aug 1, 2025
@genedna
genedna requested a review from Copilot August 1, 2025 11:59
@genedna
genedna requested a review from Ivanbeethoven August 1, 2025 11:59

This comment was marked as outdated.

@yyjeqhc yyjeqhc changed the title Fix: Add API for Orion in Scaorio and update Orion to fit the changes, allowing it to build successfully. Fix: Add API for Orion in Scarpio and update Orion to fit the changes, allowing it to build successfully. Aug 1, 2025
@yyjeqhc yyjeqhc changed the title Fix: Add API for Orion in Scarpio and update Orion to fit the changes, allowing it to build successfully. Fix: Add API for Orion in Scorpio and update Orion to fit the changes, allowing it to build successfully. Aug 2, 2025
@genedna
genedna requested a review from Copilot August 2, 2025 05:28

This comment was marked as outdated.

This comment was marked as outdated.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds asynchronous mount API functionality to Scorpio and updates Orion to handle the new asynchronous mount workflow. The changes enable Orion to initiate mount requests through Scorpio's API and poll for completion status rather than waiting synchronously.

Key changes:

  • Modified Scorpio's mount handler to return immediately with a request ID and track mount operations asynchronously
  • Added new /api/fs/select endpoint in Scorpio for querying mount task status
  • Updated Orion's buck controller to use the new asynchronous mount API with polling mechanism

Reviewed Changes

Copilot reviewed 15 out of 18 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
scorpio/src/daemon/mod.rs Major refactor to implement asynchronous mount operations with task tracking and status querying
scorpio/scorpio.toml Configuration change to reduce load directory depth
orion/src/buck_controller.rs Updated mount function to use async API with polling and better error handling
orion/Cargo.toml Added JSON feature to reqwest dependency
orion/.env Updated environment configuration paths
Other files Code formatting and import organization improvements

Comment thread scorpio/src/daemon/mod.rs Outdated
}

// Invalid request: neither request_id nor path provided
// TODO: Consider implementing cleanup for completed tasks to prevent memory leaks

Copilot AI Aug 3, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The TODO comment indicates a memory leak concern. The tasks DashMap will grow indefinitely as completed tasks are never removed. Consider implementing a cleanup mechanism or TTL for completed tasks to prevent unbounded memory growth.

Copilot uses AI. Check for mistakes.
Comment thread orion/src/buck_controller.rs Outdated
Comment on lines +59 to +62
let max_attempts = 30;

for _attempt in 1..=max_attempts {

Copilot AI Aug 3, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The hardcoded max_attempts value of 30 should be configurable or at least defined as a named constant to improve maintainability and allow for easier tuning.

Suggested change
let max_attempts = 30;
for _attempt in 1..=max_attempts {
for _attempt in 1..=MAX_ATTEMPTS {

Copilot uses AI. Check for mistakes.
Comment thread orion/src/buck_controller.rs Outdated

for _attempt in 1..=max_attempts {
// Wait before checking status
sleep(Duration::from_secs(10)).await;

Copilot AI Aug 3, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The hardcoded 10-second polling interval should be configurable or defined as a named constant to improve maintainability and allow for different polling strategies.

Suggested change
sleep(Duration::from_secs(10)).await;
sleep(Duration::from_secs(MOUNT_POLL_INTERVAL_SECS)).await;

Copilot uses AI. Check for mistakes.
Comment thread scorpio/src/daemon/mod.rs Outdated
}

// Secondary lookup method: search by path
// This is less efficient as it requires iterating through all tasks

Copilot AI Aug 3, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The path-based lookup requires O(n) iteration through all tasks. Consider adding a secondary index (path -> request_id mapping) to improve lookup performance, especially as the number of concurrent tasks grows.

Copilot uses AI. Check for mistakes.
@Ivanbeethoven

Copy link
Copy Markdown
Collaborator

LGTM

@Ivanbeethoven
Ivanbeethoven added this pull request to the merge queue Aug 3, 2025
Merged via the queue into gitmono-dev:main with commit 2fd4e9a Aug 3, 2025
8 of 13 checks passed
@yyjeqhc
yyjeqhc deleted the fix_orion_build branch August 3, 2025 09:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants